monitor-server/src/test/java/org/gridsuite/monitor/server/services/MonitorServiceTest.java (1)

301-335: Align test data with production expectations (resultType).

Both steps include result IDs but omit resultType, which can mask real‑world behavior. Add resultType to each step so the test reflects actual data contracts.

♻️ Proposed tweak

         ProcessExecutionStepEntity step0 = ProcessExecutionStepEntity.builder()
                 .id(UUID.randomUUID())
                 .stepOrder(0)
                 .reportId(reportId1)
                 .resultId(resultId1)
+                .resultType(ResultType.SECURITY_ANALYSIS)
                 .build();
@@
         ProcessExecutionStepEntity step1 = ProcessExecutionStepEntity.builder()
                 .id(UUID.randomUUID())
                 .stepOrder(1)
                 .reportId(reportId2)
                 .resultId(resultId2)
+                .resultType(ResultType.SECURITY_ANALYSIS)
                 .build();

monitor-server/src/main/java/org/gridsuite/monitor/server/services/SecurityAnalysisService.java (1)

57-66: Implementation looks good and follows existing patterns.

The method correctly:

• Logs the deletion action consistently with getResult
• Constructs the URI with query params (the batch-style resultsUuids parameter aligns with the API design)
• Uses restTemplate.delete() which will propagate HTTP errors as exceptions

One consideration for resilience: external calls without timeouts or explicit error handling can cause cascading failures if the security-analysis-server becomes slow or unavailable. Since getResult also lacks this, it might be worth addressing both in a follow-up for improved operational resilience.

monitor-server/src/main/java/org/gridsuite/monitor/server/services/MonitorService.java (1)

151-168: Avoid mutable out-parameters for deletion targets.

deleteExecutionEntity mutates caller-provided lists, which makes the public API easy to misuse (e.g., passing List.of(...) will throw) and hides side effects. Consider returning a small value object (e.g., a record with result/report IDs) or narrowing visibility to reduce accidental misuse.

monitor-server/src/test/java/org/gridsuite/monitor/server/MonitorIntegrationTest.java (1)

86-113: Add a userId persistence assertion.
The test now passes a userId but doesn’t verify it was stored on the execution entity.

🧪 Suggested assertion

         assertThat(execution.getStatus()).isEqualTo(ProcessStatus.SCHEDULED);
         assertThat(execution.getSteps()).isEmpty();
+        assertThat(execution.getUserId()).isEqualTo(userId);

monitor-server/src/main/resources/db/changelog/changesets/changelog_20260130T160426Z.xml (1)

44-60: Consider an index for launched‑process queries.
findByTypeAndStartedAtIsNotNullOrderByStartedAtDesc will scan on (type, started_at) as data grows. Adding an index on those columns would keep this query fast.

monitor-server/src/main/java/org/gridsuite/monitor/server/services/MonitorService.java (3)

161-164: Add @Transactional(readOnly = true) for read consistency.

This method queries the repository and maps entities to DTOs. For consistency with similar read methods like getReports (Line 127) and getResults (Line 144), consider adding the @Transactional(readOnly = true) annotation.

♻️ Suggested fix

+    `@Transactional`(readOnly = true)
     public List<ProcessExecution> getLaunchedProcesses(ProcessType processType) {
         return executionRepository.findByTypeAndStartedAtIsNotNullOrderByStartedAtDesc(processType.name()).stream()
             .map(processExecutionMapper::toDto).toList();
     }

---

166-182: Consider returning a result object instead of mutating parameter lists.

The method uses out-parameters (resultIds, reportIds) which is an unusual pattern in Java. Returning a dedicated result object or record would be more idiomatic and easier to reason about.

♻️ Example refactor using a record

+    public record ExecutionRelatedIds(List<ResultInfos> resultIds, List<UUID> reportIds) {}
+
     `@Transactional`(readOnly = true)
-    public boolean getReportsAndResultsUuids(UUID executionId, List<ResultInfos> resultIds, List<UUID> reportIds) {
+    public Optional<ExecutionRelatedIds> getReportsAndResultsUuids(UUID executionId) {
         Optional<ProcessExecutionEntity> executionEntity = executionRepository.findById(executionId);
-        if (executionEntity.isPresent()) {
-            executionEntity.get().getSteps().forEach(step -> {
-                if (step.getResultId() != null && step.getResultType() != null) {
-                    resultIds.add(new ResultInfos(step.getResultId(), step.getResultType()));
-                }
-                if (step.getReportId() != null) {
-                    reportIds.add(step.getReportId());
-                }
-            });
-            return true;
-        } else {
-            return false;
-        }
+        return executionEntity.map(entity -> {
+            List<ResultInfos> results = new ArrayList<>();
+            List<UUID> reports = new ArrayList<>();
+            entity.getSteps().forEach(step -> {
+                if (step.getResultId() != null && step.getResultType() != null) {
+                    results.add(new ResultInfos(step.getResultId(), step.getResultType()));
+                }
+                if (step.getReportId() != null) {
+                    reports.add(step.getReportId());
+                }
+            });
+            return new ExecutionRelatedIds(results, reports);
+        });
     }

---

189-200: External deletion failures leave the execution intact but resources partially deleted.

The current flow deletes external resources (results, reports) before the DB entity. If an external deletion fails mid-way, some resources may be deleted while others remain, and the execution entity is not removed. Consider:

1. Treating 404 responses from external services as success (resource already gone).
2. Aggregating failures and continuing to attempt all deletions before throwing.
3. Using a cleanup/outbox pattern for reliability.

This is a reliability concern but may be acceptable depending on operational requirements.

monitor-server/src/test/java/org/gridsuite/monitor/server/services/ProcessConfigServiceTest.java (1)

109-130: Test relies on mocked entity state rather than verifying repository save.

In updateSecurityAnalysisConfig, after calling updateProcessConfig, the test calls getProcessConfig which returns the same mocked entity. This works because the @Spy mapper mutates the entity in-place, but it doesn't verify that processConfigRepository.save() was called. Consider adding a verification if the service is expected to explicitly save updates.

monitor-server/src/main/java/org/gridsuite/monitor/server/controllers/ProcessConfigController.java (1)

45-51: Consider returning HTTP 201 Created for resource creation.

The createProcessConfig endpoint returns HTTP 200, but RESTful conventions suggest using HTTP 201 (Created) when a new resource is successfully created. You could also add a Location header pointing to the new resource.

♻️ Suggested fix

     `@PostMapping`(value = "", consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
     `@Operation`(summary = "Create process config")
     `@ApiResponses`(value = {
-        `@ApiResponse`(responseCode = "200", description = "process config was created")})
+        `@ApiResponse`(responseCode = "201", description = "process config was created")})
     public ResponseEntity<UUID> createProcessConfig(`@RequestBody` ProcessConfig processConfig) {
-        return ResponseEntity.ok().body(processConfigService.createProcessConfig(processConfig));
+        UUID id = processConfigService.createProcessConfig(processConfig);
+        return ResponseEntity.status(HttpStatus.CREATED).body(id);
     }

Note: You'll need to import org.springframework.http.HttpStatus.

monitor-server/src/main/java/org/gridsuite/monitor/server/services/ProcessConfigService.java (2)

44-51: Exception in flatMap for unknown entity types may be unexpected.

If an entity exists in the database with an unsupported type, getProcessConfig throws IllegalArgumentException instead of returning Optional.empty(). This is reasonable if unknown types indicate a bug, but consider whether returning empty and logging a warning might be more graceful for forward compatibility.

---

70-77: Minor: existsById + deleteById results in two DB round-trips.

This could be optimized to a single operation, though the current approach is clearer and the performance impact is likely negligible for this use case.

♻️ Alternative using repository's delete count

If your repository extends JpaRepository, you could use a custom method that returns the delete count, or simply call deleteById and handle EmptyResultDataAccessException (though this is deprecated in recent Spring Data). The current approach is acceptable.

monitor-server/src/test/java/org/gridsuite/monitor/server/controllers/MonitorControllerTest.java (1)

122-139: Unnecessary userId header in getLaunchedProcesses test.

The GET /executions endpoint doesn't require or use the userId header based on the controller code. Including .header("userId", "user1,user2,user3") in the test is misleading and unnecessary.

♻️ Suggested fix

-        mockMvc.perform(get("/v1/executions?processType=SECURITY_ANALYSIS").accept(MediaType.APPLICATION_JSON_VALUE).header("userId", "user1,user2,user3"))
+        mockMvc.perform(get("/v1/executions?processType=SECURITY_ANALYSIS").accept(MediaType.APPLICATION_JSON_VALUE))
             .andExpect(status().isOk())

monitor-server/src/main/java/org/gridsuite/monitor/server/entities/AbstractProcessConfigEntity.java (1)

52-58: Consider lazy loading for modificationUuids if the collection can grow large.

FetchType.EAGER on @ElementCollection loads all modification UUIDs whenever the entity is fetched. If this list remains small (a few items), EAGER is fine. However, if it could grow to hundreds of entries, switching to FetchType.LAZY and fetching on demand would reduce memory pressure and improve query performance.

monitor-server/src/main/java/org/gridsuite/monitor/server/entities/SecurityAnalysisConfigEntity.java (2)

36-40: @AllArgsConstructor only covers fields in this class, not inherited ones.

Lombok's @AllArgsConstructor generates a constructor with only parametersUuid and contingencies, excluding id and modificationUuids from the parent AbstractProcessConfigEntity. If you need to construct an entity with all fields (including inherited ones), consider using a builder pattern with @SuperBuilder on both parent and child, or a manual constructor.

♻️ Suggested approach using `@SuperBuilder`

In AbstractProcessConfigEntity:

+import lombok.experimental.SuperBuilder;
+
 `@Entity`
 `@Table`(name = "process_config")
 `@Inheritance`(strategy = InheritanceType.JOINED)
 `@DiscriminatorColumn`(name = "process_type", discriminatorType = STRING)
 `@NoArgsConstructor`
-@AllArgsConstructor
+@SuperBuilder
 `@Getter`
 `@Setter`
 public abstract class AbstractProcessConfigEntity {

In SecurityAnalysisConfigEntity:

+import lombok.experimental.SuperBuilder;
+
 `@Entity`
 `@Table`(name = "security_analysis_config")
 `@DiscriminatorValue`("SECURITY_ANALYSIS")
 `@PrimaryKeyJoinColumn`(foreignKey = `@ForeignKey`(name = "securityAnalysisConfig_id_fk_constraint"))
 `@NoArgsConstructor`
-@AllArgsConstructor
+@SuperBuilder
 `@Getter`
 `@Setter`
 public class SecurityAnalysisConfigEntity extends AbstractProcessConfigEntity {

---

44-50: Same EAGER fetch consideration as the parent class.

If contingencies can grow to a large number of items, consider FetchType.LAZY to avoid loading all contingencies on every entity fetch.

monitor-server/src/test/java/org/gridsuite/monitor/server/services/MonitorServiceTest.java (2)

56-57: Unused @Spy for ProcessExecutionMapper.

processExecutionMapper is declared as a @Spy but is never referenced in any test method. If it's intended for future use, consider adding a TODO comment. Otherwise, remove it to avoid confusion.

♻️ Remove unused spy

-    `@Spy`
-    private ProcessExecutionMapper processExecutionMapper;
-

---

372-384: Test data inconsistency: step0 has resultId but no resultType.

step0 is configured with resultId but without resultType, while step1 has both. This leads to deleteResult being called only once (Line 406). While this may correctly reflect the implementation's behavior (skip deletion when resultType is null), the test fixture is ambiguous.

Consider either:

1. Removing resultId from step0 if it's intentionally incomplete, or
2. Adding resultType to step0 and updating the verification to times(2) if both should have deletable results.

♻️ Option 1: Remove resultId from step0 for clarity

         ProcessExecutionStepEntity step0 = ProcessExecutionStepEntity.builder()
             .id(UUID.randomUUID())
             .stepOrder(0)
             .reportId(reportId1)
-            .resultId(resultId1)
             .build();

And update:

-        UUID resultId1 = UUID.randomUUID();

monitor-server/src/test/java/org/gridsuite/monitor/server/services/MonitorServiceTest.java (1)

371-376: Test implicitly validates null resultType filtering—consider making it explicit.

step0 lacks a resultType, so the implementation correctly skips its result deletion. The times(1) verification on line 403 passes because only step1 (with resultType) triggers deleteResult.

For clarity, consider verifying the exact ResultInfos argument:

verify(resultService).deleteResult(new ResultInfos(resultId2, ResultType.SECURITY_ANALYSIS));

This makes the test's intent explicit and guards against accidentally deleting the wrong result.

monitor-server/src/main/java/org/gridsuite/monitor/server/controllers/MonitorController.java (1)

86-94: Minor inconsistency: missing @Parameter annotation on executionId.

Other endpoints in this controller (e.g., getExecutionReports, getExecutionResults, getStepsInfos) include @Parameter(description = "Execution UUID") on the executionId path variable. Consider adding it here for consistent API documentation.

📝 Suggested fix

     `@DeleteMapping`("/executions/{executionId}")
     `@Operation`(summary = "Delete an execution")
     `@ApiResponses`(value = {`@ApiResponse`(responseCode = "200", description = "Execution was deleted"),
         `@ApiResponse`(responseCode = "404", description = "Execution was not found")})
-    public ResponseEntity<Void> deleteExecution(`@PathVariable` UUID executionId) {
+    public ResponseEntity<Void> deleteExecution(`@Parameter`(description = "Execution UUID") `@PathVariable` UUID executionId) {
         return monitorService.deleteExecution(executionId) ?
             ResponseEntity.ok().build() :
             ResponseEntity.notFound().build();
     }